Stream active notification content attributes in recorder-safe buckets#6801
Stream active notification content attributes in recorder-safe buckets#6801Komzpa wants to merge 3 commits into
Conversation
e12a782 to
d6f6e19
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the active notification count sensor to default to not collecting notification content attributes, and migrates existing installs to disable the setting and clear any cached notification content.
Changes:
- Change the active notification content-attributes setting default from
truetofalse. - Add a Room database migration (51 → 52) that disables the setting, deletes cached attributes, and forces a resend.
- Add an Android migration test and bump Room schema/version to 52.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| common/src/main/res/values/strings.xml | Updates the sensor description text to remove the “enabled by default” claim. |
| common/src/main/kotlin/io/homeassistant/companion/android/database/migration/DatabaseMigration.kt | Adds migration 51→52 to disable the setting, clear stored attributes, and reset last-sent state/icon. |
| common/src/main/kotlin/io/homeassistant/companion/android/database/AppDatabase.kt | Bumps Room DB version to 52. |
| common/src/androidTest/kotlin/io/homeassistant/companion/android/database/migration/AppDatabaseMigrationTest.kt | Adds an instrumentation test validating the 51→52 migration behavior. |
| common/schemas/io.homeassistant.companion.android.database.AppDatabase/52.json | Adds the Room schema snapshot for version 52. |
| app/src/main/kotlin/io/homeassistant/companion/android/sensors/NotificationSensorManager.kt | Changes the runtime default for including notification contents as attributes to false. |
| applicationContext, | ||
| activeNotificationCount, | ||
| SETTING_INCLUDE_CONTENTS_AS_ATTRS, | ||
| default = true, |
There was a problem hiding this comment.
That's a breaking change, @jpelgrom how do you handle such thing in the past? Users relying on this at the moment will have to go in the settings and enable it back. It might break some automations.
There was a problem hiding this comment.
Adjusted this so it no longer disables the setting for existing users or changes the default to off.
The current version keeps active_notification_count_content_attrs enabled by default and removes the 51 → 52 migration entirely. Instead, when the notification content payload is too large for recorder, the app sends the same count across multiple forced bucket updates with short part / parts / snapshot metadata. That keeps existing automations from losing the content attributes while keeping each recorded state payload below the recorder size limit.
There was a problem hiding this comment.
That's a breaking change, @jpelgrom how do you handle such thing in the past? Users relying on this at the moment will have to go in the settings and enable it back. It might break some automations.
Generally we don't touch settings for sensors once created. When changing it in code it won't change it in the database by default, unless you add a migration as was done here before the entire overhaul.
Komzpa
changed the title
|
Personally speaking, this bucket behavior is very convoluted and once again highlights how this shouldn't be accommodated in a sensor. It also duplicates sending logic in the actual sensor which is unwanted. |
Summary
Why
When one device has many active notifications, sending every notification as attributes on a single state update can exceed Home Assistant recorder's state-attribute size limit. In that case recorder warns and does not store the oversized attributes.
This keeps the numeric notification count stable while sending the content attributes as multiple forced updates. Each update has the same total count and a small part marker, so recorder can store the full snapshot across history rows. Buckets are sent oldest first; the last/current bucket contains the newest notifications.
Bucket metadata uses short keys:
total: total active notificationspart: current bucket number, starting at 1parts: total bucket countlast: whether this is the final bucketsnapshot: shared id for all buckets from one updatecut/cut_count: present only if oversized content had to be trimmedTesting
git diff --check./gradlew :app:ktlintCheck :common:ktlintCheck./gradlew :common:compileDebugAndroidTestKotlin./gradlew :app:testFullDebugUnitTest --tests 'io.homeassistant.companion.android.sensors.NotificationSensorManagerTest'